Skip to content

Remove even more usage of clownshoes in symbols #9035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

alexcrichton
Copy link
Member

This removes another large chunk of this odd 'clownshoes' identifier showing up
in symbol names. These all originated from external crates because the encoded
items were encoded independently of the paths calculated in ast_map. The
encoding of these paths now uses the helper function in ast_map to calculate the
"pretty name" for an impl block.

Unfortunately there is still no information about generics in the symbol name,
but it's certainly vastly better than before

hash::__extensions__::write::_version::v0.8

becomes

hash::Writer$SipState::write::hversion::v0.8

This also fixes bugs in which lots of methods would show up as meth_XXX, they
now only show up as meth and throw some extra characters onto the version
string.

This removes another large chunk of this odd 'clownshoes' identifier showing up
in symbol names. These all originated from external crates because the encoded
items were encoded independently of the paths calculated in ast_map. The
encoding of these paths now uses the helper function in ast_map to calculate the
"pretty name" for an impl block.

Unfortunately there is still no information about generics in the symbol name,
but it's certainly vastly better than before

    hash::__extensions__::write::_version::v0.8

becomes

    hash::Writer$SipState::write::hversion::v0.8

This also fixes bugs in which lots of methods would show up as `meth_XXX`, they
now only show up as `meth` and throw some extra characters onto the version
string.
@bluss
Copy link
Member

bluss commented Sep 7, 2013

beautiful

// Prefix with _ so that it never blends into adjacent digits
hash.unshift_char('_');
// Prefix with 'h' so that it never blends into adjacent digits
hash.unshift_char('h');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why s/_/h/?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seemed a little nicer to look at rather than having ::_<hash> now it's ::h<hash>. Just my own preference though, both achieve the same purpose of preventing ::<digit><rest of hash> if the first part of the hash is a digit (I believe).

@huonw
Copy link
Member

huonw commented Sep 7, 2013

Looks fine to me.

Out of interest, what does this do for a parameterised trait?

@alexcrichton
Copy link
Member Author

Sadly it doesn't deal with them at all, HashMap<K, V> always ends up looking like hashmap::MutableMap$HashMap::insert for all variants.

I don't think that this is too bad, but it would be nice to have the type information somewhere. Thankfully impl_pretty_name is now the one place which needs to change to make this possible.

bors added a commit that referenced this pull request Sep 8, 2013
This removes another large chunk of this odd 'clownshoes' identifier showing up
in symbol names. These all originated from external crates because the encoded
items were encoded independently of the paths calculated in ast_map. The
encoding of these paths now uses the helper function in ast_map to calculate the
"pretty name" for an impl block.

Unfortunately there is still no information about generics in the symbol name,
but it's certainly vastly better than before

    hash::__extensions__::write::_version::v0.8

becomes

    hash::Writer$SipState::write::hversion::v0.8

This also fixes bugs in which lots of methods would show up as `meth_XXX`, they
now only show up as `meth` and throw some extra characters onto the version
string.
@bors bors closed this Sep 8, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants